r? @thomcc

(rust-highfive has picked a reviewer for you, use r? to override)

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

@bors try @rust-timer queue

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

⌛ Trying commit b852d54f74955dd93d08f60cb1d5f31fc17d1104 with merge a4bad50fb7be69e1d11952a70824ba9949d48ad7...

@bors try

⌛ Trying commit b3ca318d62aa09f0e6ce1e234b4932a98d53d724 with merge 3c5bba5b33e8613c13885b42641ada47186c9186...

☀️ Try build successful - checks-actions
Build commit: 3c5bba5b33e8613c13885b42641ada47186c9186 (3c5bba5b33e8613c13885b42641ada47186c9186)

Queued 3c5bba5b33e8613c13885b42641ada47186c9186 with parent e7119a0, future comparison URL.

Finished benchmarking commit (3c5bba5b33e8613c13885b42641ada47186c9186): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

Max RSS (memory usage)

Cycles

Hm, very surprising that this would have any overhead. I kind of suspect it's an artifact of the compilation process, but who knows.

Maybe it's just that the MIR for it is already pretty good. Essentially it's just

if compute_discriminant(self) == 1 {
    _0 = move ((_1 as Some).0: T);
} else {
    call std::hint::unreachable()
}

That memcpy is roughly the same to LLCM as the transmute-copy, for a niched layout, and noticing that it doesn't need the condition at all and dropping unnecessary code is already pretty cheap for LLVM.

And rust ends up emitting all the discriminant calculation LLVM-IR anyway, since folding the if away doesn't happen in the polymorphic MIR, and thus cg_llvm will still need to output that code, which seems like it loses any advantage that it might have over from the transmute being easier for it to understand.

Maybe that could be improved by making it if const { mem::size_of::<T>() == mem::size_of::<Self>() } (thanks, #96557!) and reviving some of #91222 to be smarter about the resulting constant branches in codegen.

@bors try @rust-timer queue

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

⌛ Trying commit 7157cfe with merge 24860e60db7c91164ed67469532d69c3ab700541...

Maybe it's just that the MIR for it is already pretty good. Essentially it's just

But the LLVM-IR contains an assume which aiui is not without downsides. Transmuting avoids it entirely. https://rust.godbolt.org/z/4xorq6E5T

☀️ Try build successful - checks-actions
Build commit: 24860e60db7c91164ed67469532d69c3ab700541 (24860e60db7c91164ed67469532d69c3ab700541)

Queued 24860e60db7c91164ed67469532d69c3ab700541 with parent 9a963e3, future comparison URL.

Finished benchmarking commit (24860e60db7c91164ed67469532d69c3ab700541): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

Max RSS (memory usage)

Cycles

compile-time losses are consistent with the previous run, but so are binary-size changes (especially in opt-full builds) and it is spending more time in LLVM, so it's having an effect on the optimizer, just not the one expected.

I'll take a look at the generated assembly maybe they're diffable.

This code is somewhere in RawVec, the right side is this branch.

I think llvm makes use of that assume downstream of a next_unchecked's result being re-packaged into another Option.